Apply JWT decoder timeout properties for AAD and B2C resource servers#49329
Conversation
- Apply jwtConnectTimeout/jwtReadTimeout properties to RestTemplate used by JWT decoder in AadResourceServerConfiguration and AadB2cResourceServerAutoConfiguration - Remove incorrect @deprecated annotation from timeout properties in AadAuthenticationProperties and AadB2cProperties (the deprecation was based on a RestOperations bean mechanism from PR #30456 that was later removed, making the deprecation guidance invalid) - Fix javadoc: correct default value description from '500s' to '500ms' - Add tests for default and custom timeout property values
There was a problem hiding this comment.
Pull request overview
This PR updates Spring Cloud Azure (autoconfigure) resource-server support so the jwtConnectTimeout / jwtReadTimeout properties are actually applied to the HTTP client used for JWK retrieval, and cleans up related property metadata and tests.
Changes:
- Apply configured JWT connect/read timeouts to the
RestTemplate/RestTemplateBuilderused by AAD and B2C JWT decoding/JWK retrieval. - Remove incorrect
@Deprecatedmarkers on JWT timeout/size-limit properties and fix the documented default timeout unit. - Add unit tests for default/custom timeout property binding.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aadb2c/configuration/AadB2cResourceServerAutoConfiguration.java | Applies B2C timeout properties to the RestTemplateBuilder used for JWK retrieval. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/configuration/AadResourceServerConfiguration.java | Applies AAD timeout properties to the RestTemplate passed into NimbusJwtDecoder. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aadb2c/configuration/properties/AadB2cProperties.java | Removes deprecations and updates Javadoc defaults for JWT timeout/size-limit properties. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/configuration/properties/AadAuthenticationProperties.java | Removes deprecations and updates Javadoc defaults for JWT timeout/size-limit properties. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aadb2c/configuration/AadB2cResourceServerAutoConfigurationTests.java | Adds tests for default/custom B2C timeout property values. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/configuration/AadResourceServerConfigurationTests.java | Adds tests for default/custom AAD timeout property values. |
…on timeout fields
|
/azp run java - spring - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
What's a rough timeframe for this to make it to a release of com.azure.spring spring-cloud-azure-dependencies? |
Current plan is releasing it before the end of June. |
|
@rujche I found this ticket which was closed without resolution: #42159 which made me look deeper into this. Both before and after this PR, I am not sure that the code that sets up the cache was correctly adapted to upstream changes in Nimbus, it appears to me that the relevant keys for configuring the cache ( It seems to me that we would have to call JWKSource<SecurityContext> jwkSource = JWKSourceBuilder
.create(URI.create(identityEndpoints.getJwkSetEndpoint()).toURL())
.cache(aadProps.getJwkSetCacheLifespan().toMillis(), aadProps.getJwtConnectTimeout().toMillis())
.refreshAheadCache(aadProps.getJwkSetCacheRefreshTime().toMillis(), true)
.build();
NimbusJwtDecoder nimbusJwtDecoder = NimbusJwtDecoder
.withJwkSource(jwkSource)
.build();in order to wire the If you want me to I can open a separate ticket for this? |
|
@rujche Looks good to me as a fix for the unused cache bug. Improvement suggestion: enable the use of a Bean-injected customizer for the Here's an example of how this looks with Jackson, this customizer bean is then used whenever a @Configuration
class JacksonConfig {
@Bean
JsonMapperBuilderCustomizer jacksonCustomizer() {
return builder -> builder
.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES)
.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS)
.disable(DeserializationFeature.FAIL_ON_IGNORED_PROPERTIES)
.disable(DateTimeFeature.WRITE_DATES_AS_TIMESTAMPS)
.changeDefaultPropertyInclusion(inclusion ->
inclusion.withValueInclusion(JsonInclude.Include.NON_NULL));
}
}Alternatively look at the non-deprecated mechanisms for setting up the JWK source in nimbus, there are a number of nice to have features supported there, e.g. .outageTolerant, .retrying, along with background fetching, which all serve to improve the resilience of the components that rely on this, which are currently out of users reach when using the auto-configuration provided here, meaning they would have to configure JWK source manually in order to use them. The alternative to the customizer suggestion would be to expose individual properties for each of these. |
|
Hi, @abjugard . Thanks for the review and suggestion.
In this PR (#49356), , |
Description
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines